Repository navigation
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
clawsweeper
Bot
added
P2
|
Codex review: blocked before merge. What this changesThis PR lets workspace owners permanently delete channels through the browser, API, SDK, and CLI, removing exclusively owned content and updating connected clients. Example: An owner removes #doomed while #general remains.
Review scores
ProductKind: Feature · Worth it: Needs a maintainer decision · Fix scope: Complete Merge readiness⛔ Blocked before merge - 1 item remains Keep this PR open: permanent channel deletion is absent from current main, and the implementation merits consideration, but its destructive product and public API contract still needs maintainer acceptance. Priority: P2 Decision needed
Before merge
FindingsNone. Agent review detailsHow this fits togetherBrowser, CLI, and SDK requests enter the authenticated HTTP API; SQLite or PostgreSQL transactions authorize deletion and remove channel-owned rows, then durable events update clients and queued cleanup removes exclusive upload objects. flowchart TD
A[Browser CLI and SDK] --> B[Authenticated HTTP API]
B --> C[Current owner transaction]
C --> D[SQLite or PostgreSQL]
C --> E[Durable deletion event]
C --> F[Upload cleanup queue]
E --> G[Chat and embed clients]
F --> H[Upload storage]
Technical reviewBest possible solution: Have the channel area owner accept the implemented owner-only deletion contract or specify a narrower supported scope. Do we have a high-confidence way to reproduce the issue? This is a feature request rather than a current-main bug; source confirms that existing channel archival retains history, while the supplied browser and serve evidence exercises the proposed permanent deletion. Is this the best way to solve the issue? The implementation puts authorization and deletion in the shared store boundary, reuses durable upload cleanup, and preserves existing archive behavior; accepting the new destructive capability remains a product decision. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against fe0850b11d86. Provenance checked
TestingProof path: shipped entry point. SecurityNone. EvidenceWhat I checked:
LabelsLabel changes: No label changes. Label justifications:
Rating scale6/6 🦀 challenger crab · 5/6 🦞 diamond lobster · 4/6 🐚 platinum hermit · 3/6 🦐 gold shrimp · 2/6 🦪 silver shellfish · 1/6 🧂 unranked krab. Overall follows the weaker of proof and patch quality; ✨ marks media proof (a screenshot, video, or linked artifact) that directly shows the changed behavior. WorkflowClawSweeper edits this one comment on every review. Comment HistoryReview history (36 earlier review cycles; latest 8 shown)
Reviewed October 14, 2026, 10:03 AM ET / 14:03 UTC (Revision 37). |
b6d6079 to
aa459dd
Compare
Owners delete a channel from Channel settings, the API, or the CLI after reviewing what will be removed and typing the channel name. The channel's messages, replies, reactions, pins, topics, read state, and exclusive uploads go in one transaction; a workspace-scoped channel.deleted event moves viewers out. The last channel and the Guests workspace's provisioned channels cannot be deleted. The CLI deletes only a channel named with --channel on its command line, never CLICKCLACK_CHANNEL or the saved default. On SQLite, cascading many message deletions scanned both the messages table and the FTS index once per row. Child-key indexes and a message-to-search-row map make a 10k-message channel in a 120k-message workspace delete in 0.3 s instead of 6.6 minutes, which also speeds up workspace deletion. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]>
aa459dd to
c6d1ef8
Compare
clawsweeper
Bot
added
rating: 🐚 platinum hermit
clawsweeper
Bot
added
merge-risk: 🚨 compatibility
clawsweeper
Bot
added
merge-risk: 🚨 security-boundary
|
Updated this PR to
All seven focused Chromium cases pass. The PR body now embeds inspected before/after captures from the actual synthetic server, plus readable SQLite and PostgreSQL v0.7.0 upgrade transcripts. Those native serve runs also prove moderator and former-owner 403 preserve channel/message/upload rows, exact exclusive file bytes, cleanup jobs, and events; the committed current owner then succeeds with 204, while another channel/file/search and the earlier event cursor survive. The canonical helper completed all four passes with no P0–P2 findings, and the client repair received separate manual review. Source reviews and these fixtures do not grant product/API acceptance or imply a deployment. Permanent deletion remains a maintainer decision; I have left that merge gate open. |
clawsweeper
Bot
added
rating: 🦐 gold shrimp
|
The latest review identifies permanent-deletion acceptance as the remaining product gate. I recommend accepting the implemented owner-only contract: preview, exact-name browser confirmation, final-channel protection, transactional current-owner authorization, retained-file handling, and retirement of open clients/embeds. I also removed the low-value helper-only comparison test noted in that review ( Could the channel area owner record acceptance of this contract, or specify the concrete scope to narrow? The native SQLite/PostgreSQL upgrade and denial/allowed final-effect transcripts and inspected browser captures are linked in the PR body. That product decision remains open; I am not treating automated review as approval. |
This branch has not been deployed
1 participant
Maintainer edits
MUST: Keep Allow edits from maintainers enabled for this PR.
What Problem This Solves
Archiving hides a channel while retaining its history. Owners need an explicit way to remove an unwanted channel and its exclusively owned files permanently.
User Impact
User impact: owners review the affected messages and files, type the exact channel name, and delete it. People viewing that channel move to a remaining channel with a notice. Archive remains available. The final channel cannot be deleted; moderators, members, bot tokens, and former owners cannot delete one.
Permanent deletion remains a product and public API decision for maintainers before merge.
Why This Change Was Made
The authenticated store transaction checks current ownership, removes channel-owned rows, and publishes one durable workspace event. Exclusive file deletion uses durable cleanup and rechecks references, so content retained by another channel survives. SQLite search row IDs make the cascade bounded while preserving search on retained messages and old event cursors.
Clients retain deletion facts and filter late channel-list and search-page responses, including IDs not yet in their sidebar. The HTTP response and its event are reconciled once, preserving the original notice. Already-open channel and thread embeds retire reads, sends, sockets, history, and composers, then show a terminal deleted state.
Evidence
Refreshed onto upstream main, retaining its owner-only channel administration, current SQLC, push/sidebar support, and embedded asset generation. Migration filenames retain their original identities for earlier feature builds.
The official v0.7.0 and latest v0.7.1 binaries populated independent SQLite and PostgreSQL fixtures with channels, root and thread messages, searchable history, and exclusive/retained uploads. Restarting the candidate on the same databases produced these results through the shipped
clickclack serveentry point:channel.deletedShared store regressions and HTTP/CLI deletion checks pass, including PostgreSQL locking/concurrency and moderator/ownership-transfer rejection with the current-owner positive control. Web/SDK/embedded build and workspace typechecks pass.
Seven focused Chromium regressions pass: exact-name owner confirmation and original notice, member evacuation, final-channel rejection, held stale channel list, held first/next search pages, and already-open channel/thread embeds. A separate actual-server capture confirmed both before/after views below. All six images were inspected; they contain only synthetic Local Captain fixtures. T3 preview explicitly reported no connected browser host, so repository Playwright supplied the browser proof. No deployment is claimed.
Actual sanitized SQLite upgrade transcript · PostgreSQL transcript · Evidence provenance
The canonical autoreview helper completed all four passes against the complete candidate with no P0–P2 findings. Independent manual review of the client reconciliation and embed retirement also found no actionable issue. These source reviews do not grant maintainer acceptance of permanent deletion, which remains open.
Latest stable repeat: v0.7.1 SQLite native transcript · PostgreSQL native transcript. The final candidate binary was rebuilt for this repeat; all retained-state and current-ownership final effects passed, and private PostgreSQL fixtures were stopped.
Exact-name confirmation is covered through the shipped browser: the delete button remains disabled for an incomplete name and becomes enabled for the exact name. The redundant helper-only comparison test has been removed; the seven browser regressions remain. This follow-up changes only test inventory, so the linked serve, upgrade and browser evidence still describes the current production source. Current-head CI must finish independently.